-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Gate Codex skills flag on CLI support #6520
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
The Codex CLI removed the `skills` feature flag in newer versions , causing `goose configure` to fail with exit code 1 when it unconditionally passes `--enable/--disable skills`. This commit updates the logic to check `codex features list` before applying these flags. This ensures the flags are only passed when the installed CLI explicitly advertises support for them. While simply removing the flag usage was an option, this approach is designed to maintain backward compatibility for environments where the CLI version is pinned (e.g., in automation workflows). This also updates the documentation to clarify that the `skills` flag logic is legacy behavior for older CLIs. Signed-off-by: Yusuke Shimizu <[email protected]>
4e2b015 to
468d8f8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses a breaking change in newer versions of the Codex CLI where the skills feature flag was removed (openai/codex#8850). The implementation adds backward compatibility by detecting whether the installed Codex CLI supports the skills feature flag via codex features list and only passing --enable/--disable skills flags when supported.
Changes:
- Added runtime feature detection to check if the Codex CLI supports the
skillsfeature flag - Updated documentation to clarify that
CODEX_ENABLE_SKILLSis a legacy toggle only respected on older CLIs - Added test coverage for the feature detection logic
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| crates/goose/src/providers/codex.rs | Added skills_feature_flag_supported field, feature detection logic via codex features list, and conditional flag passing |
| documentation/docs/guides/cli-providers.md | Updated documentation to clarify skills support is native in newer CLIs and the env var is legacy-only |
| fn feature_list_contains_feature(stdout: &str, feature: &str) -> bool { | ||
| // `codex features list` output is a whitespace-separated sequence of: | ||
| // `<name> <stability> <enabled>` repeated, e.g.: | ||
| // `undo stable false shell_tool stable true ...` | ||
| // | ||
| // We match by token to be robust to both newline- and space-delimited output. | ||
| stdout | ||
| .split_whitespace() | ||
| .collect::<Vec<_>>() | ||
| .windows(3) | ||
| .any(|w| { | ||
| w[0] == feature | ||
| && matches!(w[1], "stable" | "beta" | "experimental") | ||
| && matches!(w[2], "true" | "false") | ||
| }) | ||
| } |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add test cases for edge scenarios: empty stdout, feature name appearing but with invalid stability/enabled values, feature name as substring of another word, and stdout with fewer than 3 tokens. The windows(3) call could panic on short input if the slice has fewer than 3 elements.
| stdout | ||
| .split_whitespace() | ||
| .collect::<Vec<_>>() | ||
| .windows(3) | ||
| .any(|w| { | ||
| w[0] == feature | ||
| && matches!(w[1], "stable" | "beta" | "experimental") | ||
| && matches!(w[2], "true" | "false") | ||
| }) |
Copilot
AI
Jan 16, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If stdout has fewer than 3 tokens after split_whitespace(), the windows(3) iterator will be empty (not panic), but this edge case should be tested. An empty or malformed output from codex features list would silently return false, which is correct behavior but should be explicitly verified in tests.
| stdout | |
| .split_whitespace() | |
| .collect::<Vec<_>>() | |
| .windows(3) | |
| .any(|w| { | |
| w[0] == feature | |
| && matches!(w[1], "stable" | "beta" | "experimental") | |
| && matches!(w[2], "true" | "false") | |
| }) | |
| let mut tokens = stdout.split_whitespace(); | |
| loop { | |
| let name = match tokens.next() { | |
| Some(t) => t, | |
| None => break, | |
| }; | |
| let stability = match tokens.next() { | |
| Some(t) => t, | |
| None => break, | |
| }; | |
| let enabled = match tokens.next() { | |
| Some(t) => t, | |
| None => break, | |
| }; | |
| if name == feature | |
| && matches!(stability, "stable" | "beta" | "experimental") | |
| && matches!(enabled, "true" | "false") | |
| { | |
| return true; | |
| } | |
| } | |
| false |
- Verify behavior when skills feature is not supported. - Ensure correct arguments are returned for skills flag. Signed-off-by: Yusuke Shimizu <[email protected]>
|
@michaelneale I would appreciate your review on this. |
michaelneale
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice - nice catch BTW.
Also - goose now can use codex via oauth - so doens't have to use cli if you don't wnat it
|
I think we can just drop the backwards compatibility, so cloned this, merged in main now at: #6775 thanks for doing this though! |
Summary
Addresses compatibility issues with newer versions of the Codex CLI where the
skillsfeature flag has been removed (referenced in openai/codex#8850 ).Previously,
goose configurewould fail with an exit code 1 on newer CLIs because it unconditionally passed the--enable/--disable skillsflags.While simply removing the flag checks was an option, this implementation is explicitly designed to maintain backward compatibility. This ensures stability for automation workflows and environments where CLI versions may be pinned.
Type of Change
AI Assistance
Testing
This change has been tested manually with both new and legacy versions of the Codex CLI to ensure robustness.
Screenshots/Demos (for UX changes)
Before (Crash on Codex CLI 0.85.0):
Before (Success on Codex CLI 0.77.0):
After (Backward Compatibility on Codex CLI 0.77.0):